fix(memory-ingest): stop dropping capture_origin at ingest (issue #368) - #410
Conversation
mcp_server/core/memory_ingest.py::ingest_memory accepted a caller-supplied
capture_origin field but never forwarded it to store.insert_memory, unlike
every other metadata field (source, tags, heat, ...). Every ingest_memory
caller therefore fell through to insert_memory's "unknown" default
regardless of what it intended — a silently lost field, not a harness gap.
This is why the trust-factor calibration's gated arm
(docs/provenance/trust-factor-calibration.md) could not discriminate W: the
LME/LoCoMo/BEAM benchmarks all landed on capture_origin='unknown', which
core.capture_origin.trust_factor demotes uniformly, and a uniform multiplier
cannot change WRRF order — hence LME identical to four decimals across
W in {1.0, 0.7, 0.6, 0.5}.
Fixes:
- memory_ingest.py: pass capture_origin through, default unchanged
("unknown" for callers that omit it — no behavior change elsewhere).
- benchmarks/lib/capture_origin_mix.py (new): the realistic production
mixture of capture_origin values (local_action .966 / network .025 /
deliberate .009), sourced from a measurement over this machine's own
886 Claude Code session transcripts (~/.claude/projects/**/*.jsonl) —
the only representative "existing store" available; the local memory.db
predates the capture_origin column. Deterministic sampler.
- benchmarks/lib/bench_db.py: wires the mixture into load_memories() via
_apply_capture_origin_mix(), so LME/LoCoMo/BEAM now insert a realistic
blend of trusted/untrusted origins instead of an all-unknown corpus.
16 new tests (tests_py/core/test_memory_ingest_capture_origin.py,
tests_py/benchmarks/test_capture_origin_mix.py,
tests_py/benchmarks/test_bench_db_capture_origin.py). Full suite:
7291 passed, 142 skipped, 0 failed.
What this does NOT do yet: re-run the 5-cell W sweep
(benchmarks/trust_factor_sweep.sh) under the now-mixed origins. W=0.7
remains the production value, still resting on the invalidated gated arm
documented in docs/provenance/trust-factor-calibration.md. That re-measure
is next, separately, once its cost (Docker, ~7h across 5 sequential cells)
is authorized — or a cheaper discriminating measurement is found.
Co-Authored-By: Claude <noreply@anthropic.com>
benchmarks/lib/bench_db.py hard-imports PgMemoryStore (-> psycopg) at
module level (established convention, tests_py/benchmarks/
test_lib_init_no_psycopg.py). This test file imported it unconditionally,
which broke collection on the SQLite-only CI lane
("ModuleNotFoundError: No module named 'psycopg'").
Fix: pytest.importorskip("psycopg", ...) before the benchmarks.lib.bench_db
import, matching the pattern already used by
tests_py/integration/test_recall_trust_ranking.py. Verified locally by
poisoning sys.modules for psycopg/psycopg_pool/pgvector — the suite now
skips cleanly instead of erroring, and still passes normally with psycopg
present.
Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-10 incident: a 5-cell trust-factor sweep ran while three other agents' full pytest suites were active on the same 10-core machine (load average ~11-14). Cell W=1.0 crashed on a native fatal error (rc=120, truncated CPython fatal-error signature) -- a visible failure. But a cell that merely FINISHED under the same contention (saturated connection pool, cold cache, GC pressure) would have produced a plausible-looking degraded number with nothing in the artifact to distinguish it from a clean run. The whole grid was discarded and re-run rather than salvaged, per this project's own rule: a measurement from a harness carrying a known defect is invalid and gets redone, not patched after the fact -- and machine contention during a benchmark run is exactly such a defect. Fix: benchmarks/lib/write_manifest.py::machine_load_snapshot() records load average (1/5/15 min, os.getloadavg()), cpu_count, concurrent pytest process count, and concurrent Docker container count -- wired into build_manifest() as `machine_load`, placed alongside `git_sha` in every MANIFEST.json this harness writes. Best-effort like every other field in this module (embedding_model_revision, reranker_state): a failed probe records None rather than aborting manifest generation. 6 new tests (tests_py/benchmarks/test_write_manifest_machine_load.py). Verified importable and passing with psycopg poisoned out of sys.modules (matches this module's existing psycopg-independence). Co-Authored-By: Claude <noreply@anthropic.com>
Follow-up to aa093d2's machine_load_snapshot: that fix captured load only at the END of a reproduce.sh run (inside write_manifest). A single end-of-run reading cannot distinguish "this cell ran under contention the whole time" from "load spiked right at the end" -- exactly the ambiguity that forced discarding a whole 5-cell sweep rather than salvaging the cells that merely finished (vs. the one that visibly crashed). Fix: benchmarks/lib/write_manifest.py::write_start_snapshot() captures the same machine_load_snapshot() and writes it to RESULTS_DIR/START_SNAPSHOT.json. benchmarks/reproduce.sh calls it first thing in main(), before start_db, so it predates even the container/DB overhead. build_manifest() reads that file back at cell end and folds it into MANIFEST.json as machine_load_at_start, alongside the renamed machine_load_at_end (was machine_load). START_SNAPSHOT.json is excluded from the results_files list. 10 tests (was 6), covering both snapshot points and the results_files exclusion. ruff clean, bash -n clean on reproduce.sh, verified end-to-end against a throwaway results dir. Co-Authored-By: Claude <noreply@anthropic.com>
…(CI) Test (SQLite backend) failed on GitHub's Linux CI runner: TestMachineLoadSnapshot::test_pytest_process_count_sees_the_process_running_this_test -- assert 0 >= 1. Not a flaky test: machine_load_snapshot()'s own pytest process was genuinely undercounted. Root cause: GNU procps `ps aux` truncates the COMMAND column to $COLUMNS (default 80) when stdout is not a terminal and COLUMNS is unset -- unlike BSD ps (macOS), which does not truncate piped output by default, so this never reproduced locally. `ps aux`'s fixed-width USER/PID/%CPU/... prefix alone is close to 80 columns, and pytest's actual invocation path (interpreter + script, e.g. ".../bin/python3.12 .../bin/pytest ...") pushes the "pytest" substring well past that -- so it was silently cut off on every Linux CI run, not just this one. A real undercount in the manifest field this whole fix exists to make trustworthy, not merely a test bug. Fix: pass an explicit wide `COLUMNS` override in the `ps aux` subprocess environment -- both GNU procps and BSD ps honor it, so this is portable and harmless where it wasn't the problem (macOS). Co-Authored-By: Claude <noreply@anthropic.com>
|
ZETETIC-REVIEW: APPROVE Reviewed the diff directly, not the description. The production change is one field, and it is the right one. Backward compatibility is preserved by construction: The consequence is stated honestly in the code comment: this is why the trust-factor calibration's gated arm could not discriminate W. A one-line data-plumbing defect invalidated a measurement — worth recording at the call site, which the comment does, with a pointer to the provenance document rather than a bare assertion. The rest of the diff is test and harness surface ( CI: 22 pass, 2 skipping. No blocking findings. |
…n fix (#368, #410) The original gated arm could not discriminate W: every LME/LoCoMo/BEAM memory landed on capture_origin='unknown' (memory_ingest.py dropped the field before PR #410), so the uniform demotion left WRRF order invariant. After #410 wires a realistic capture_origin mix into the benchmark harness, re-run the same 5-cell grid (1.0/0.8/0.7/0.6/0.5) in --quick mode and publish what it shows: a real, monotonic LoCoMo relevance cost at the shipped W=0.7 (-0.0165 MRR, -2.54pp R@10 vs the W=1.0 control), with two explicit reserves — quick mode is not floor-comparable (reproduce.sh:94-95), and no measurement links the adversarial-arm defense to a real-benchmark benefit. Owner decision: W stays at 0.7, cost published as measured. Commits the artifacts (PROGRESS.json, per-cell repro_dir + MANIFEST + benchmark JSON) and the resumable per-cell sweep architecture (trust_factor_sweep.sh rewrite + benchmarks/lib/sweep_progress.py) that produced them, so the branch reproduces without depending on the worktree they were run in. Co-Authored-By: Claude <noreply@anthropic.com>
* fix(benchmarks): snapshot disk space alongside load; split under 300/40 caps Second invisible-failure vector for the trust-factor sweep (issue #368 follow-up): a neighboring session sharing this machine reported that leaked throwaway SQLite test databases (226-357 MB/run) filled the host disk to 100% on 2026-08-09, taking PostgreSQL down and starving even the Bash tool's own stdout file. A cell can finish cleanly and still return degraded numbers because its volume filled under it -- the same failure shape as CPU contention, a different resource, and it left no more trace in the artifact than contention did. Fix: benchmarks/lib/disk_space_snapshot.py (new) -- free/total bytes on the repo-root filesystem and, best-effort, Docker's storage root, via shutil.disk_usage + `docker info`. Wired into write_manifest.py exactly like machine_load_snapshot: captured at cell start (write_start_snapshot) and cell end (build_manifest), as disk_space_at_start/disk_space_at_end. Also split benchmarks/lib/machine_load_snapshot.py out of write_manifest.py: the new local CLAUDE.md Code Style gate (scripts/check_craftsmanship.py, merged to main while this work was in flight) caps methods at 40 lines and files at 300, and machine_load_snapshot() (78 lines) + build_manifest() (49 lines) both already exceeded that before this commit -- pre-existing debt this change surfaced rather than introduced, fixed here since the gate now blocks any diff touching this file until it passes. Every function in the resulting three files is under the cap; verified via `python scripts/check_craftsmanship.py <files>`. 10 tests split/added across tests_py/benchmarks/test_machine_load_snapshot.py (new), test_disk_space_snapshot.py (new), and test_write_manifest_machine_load.py (now wiring-only). 33 tests total across the six touched files, all green. ruff clean. Co-Authored-By: Claude <noreply@anthropic.com> * docs(trust-factor): publish honest re-measurement after capture_origin fix (#368, #410) The original gated arm could not discriminate W: every LME/LoCoMo/BEAM memory landed on capture_origin='unknown' (memory_ingest.py dropped the field before PR #410), so the uniform demotion left WRRF order invariant. After #410 wires a realistic capture_origin mix into the benchmark harness, re-run the same 5-cell grid (1.0/0.8/0.7/0.6/0.5) in --quick mode and publish what it shows: a real, monotonic LoCoMo relevance cost at the shipped W=0.7 (-0.0165 MRR, -2.54pp R@10 vs the W=1.0 control), with two explicit reserves — quick mode is not floor-comparable (reproduce.sh:94-95), and no measurement links the adversarial-arm defense to a real-benchmark benefit. Owner decision: W stays at 0.7, cost published as measured. Commits the artifacts (PROGRESS.json, per-cell repro_dir + MANIFEST + benchmark JSON) and the resumable per-cell sweep architecture (trust_factor_sweep.sh rewrite + benchmarks/lib/sweep_progress.py) that produced them, so the branch reproduces without depending on the worktree they were run in. Co-Authored-By: Claude <noreply@anthropic.com> * style(benchmarks): ruff format sweep_progress.py CI Lint failure on PR #415 — sweep_progress.py was committed from the worktree without ever going through the formatter. No logic change: one line collapsed under the ruff line-length rule. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
mcp_server/core/memory_ingest.py::ingest_memoryaccepted a caller-suppliedcapture_originfield but never forwarded it tostore.insert_memory,unlike every other metadata field (
source,tags,heat, ...). Everyingest_memorycaller therefore silently fell through toinsert_memory's"unknown"default regardless of what it intended. This is a productioncode defect, not a harness gap — a field accepted then silently dropped.
This is why
docs/provenance/trust-factor-calibration.md's gated arm couldnot discriminate the trust-factor weight W: LME/LoCoMo/BEAM all landed on
capture_origin='unknown', whichcore.capture_origin.trust_factordemotesuniformly, and a uniform multiplier cannot change WRRF order — hence LME
identical to four decimals across W in {1.0, 0.7, 0.6, 0.5} in that document's
existing results table.
What's fixed
memory_ingest.py—capture_originnow passes through like every otherfield. Default unchanged (
"unknown"for callers that omit it) — nobehavior change for any existing caller that doesn't set it.
benchmarks/lib/capture_origin_mix.py(new) — the realistic productionmixture of
capture_originvalues (local_action.966 /network.025 /deliberate.009), sourced from a measurement over this machine's own 886Claude Code session transcripts (
~/.claude/projects/**/*.jsonl) — theonly representative "existing store" available (the local
memory.dbpredates the
capture_origincolumn, no live production DB touched).Deterministic sampler, documented derivation and limitation in the module
docstring.
benchmarks/lib/bench_db.py— wires the mixture intoload_memories()via_apply_capture_origin_mix(), so LME/LoCoMo/BEAM now insert a realisticblend of trusted/untrusted origins instead of an all-
unknowncorpus. Neveroverwrites an explicit origin already set by a caller (e.g. the adversarial
corpus).
What's NOT fixed yet (next, separately)
The production value W = 0.7 still rests on the invalidated gated arm
documented in
docs/provenance/trust-factor-calibration.md— that documentis unchanged by this PR. Re-running the 5-cell W sweep
(
benchmarks/trust_factor_sweep.sh) under the now-mixed origins is thedirect continuation of this same contract, not a separate task, but it costs
~7h across 5 sequential Docker-isolated
reproduce.shcells and that cost isbeing reported to the repo owner for a go/no-go before running it — plus a
check for whether the adversarial arm (
benchmarks/lib/trust_factor_sweep.py,seconds on in-memory SQLite) is now sufficient on its own, since the origins
it discriminates against are the same ones the gated arm was blind to before
this fix.
Test plan
uv run --no-sync pytest -q -p no:randomly— 7291 passed, 142 skipped,0 failed
tests_py/core/test_memory_ingest_capture_origin.py,tests_py/benchmarks/test_capture_origin_mix.py,tests_py/benchmarks/test_bench_db_capture_origin.pyruff check/ruff format --checkclean on touched filesCo-Authored-By: Claude noreply@anthropic.com